Skip to content

fix(npm): stop skipping dependency subtree when root entry has no version#423

Merged
ruromero merged 3 commits into
guacsec:mainfrom
ruromero:TC-4128
Apr 20, 2026
Merged

fix(npm): stop skipping dependency subtree when root entry has no version#423
ruromero merged 3 commits into
guacsec:mainfrom
ruromero:TC-4128

Conversation

@ruromero

@ruromero ruromero commented Apr 16, 2026 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Collaborator

Summary

  • Fix JavaScriptProvider.addDependenciesFromKey() which skipped both the entry and its entire transitive dependency subtree when a root-level dependency had no version field (e.g., file: deps, workspace packages, linked packages)
  • Now treats null version as valid (produces a versionless PURL) and always recurses into children, matching the JS client's _addDependenciesToSbom() behavior
  • Updated reproducer test comments to reflect the fix (TC-3818 / TC-4128)

Root cause

The null-version check at line 181 did return inside a forEachRemaining lambda, which exited the lambda body entirely — skipping both the sbom.addDependency() call and the addDependenciesOf() recursion for that entry's children.

Fix

Replace the early-return guard with a null-safe version extraction that passes null to toPurl() (valid per PURL spec — version is optional), then unconditionally recurses into children.

Implements TC-4128

Test plan

  • Reproducer test test_provideStack_includes_deps_of_root_entry_without_version passes (7 components)
  • All 37 JavaScript provider tests pass (npm, pnpm, yarn-classic, yarn-berry)
  • Spotless formatting clean

🤖 Generated with Claude Code

@ruromero
ruromero requested review from a-oren April 16, 2026 16:57
@ruromero

ruromero commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator Author

Verification Report for TC-4128 (commit 154847e)

Check Result Details
Review Feedback PASS 1 comment (ruromero) classified as nit — informational confirmation of approach. No code changes requested
Root-Cause Investigation N/A No sub-tasks created
Scope Containment PASS All 5 changed files match task scope (1 source file + 1 test file + 3 fixtures)
Diff Size PASS +145 / -4 across 5 files — appropriate for bug fix with test and fixtures
Commit Traceability PASS Both commits reference TC-4128
Sensitive Patterns PASS No sensitive patterns detected
CI Status WARN 3 gradle-groovy integration tests failed due to backend 504 timeout (rhtpa provider) — infrastructure issue, not code-related. Build and test Java 21 + all other 33 integration tests pass
Acceptance Criteria PASS 3/3 criteria met
Test Quality PASS No repetitive tests; new test has doc comment
Verification Commands PASS mvn spotless:check passes; all 37 JS provider tests pass

Overall: WARN

The only non-PASS result is the CI gradle-groovy integration test failure, which is caused by a transient 504 timeout from the rhtpa staging backend — completely unrelated to this PR's npm/pnpm fix. All unit tests, spotless checks, and 33/36 integration test suites pass.

Acceptance Criteria Detail:

  1. ✅ test_provideStack_includes_deps_of_root_entry_without_version asserts 7 components and passes
  2. ✅ All 37 existing npm/pnpm provider tests continue to pass (0 failures, 0 errors)
  3. ✅ addDependenciesFromKey() now produces a versionless PURL and always recurses into children via addDependenciesOf()

This comment was AI-generated by sdlc-workflow/verify-pr v0.6.1.

a-oren
a-oren reviewed Apr 19, 2026
View reviewed changes
Comment thread src/main/java/io/github/guacsec/trustifyda/providers/JavaScriptProvider.java
ruromero added 3 commits April 20, 2026 14:17
@ruromero
fix(npm): stop skipping dependency subtree when root entry has no ver…
435e32d 
…sion

In addDependenciesFromKey(), a null version check caused an early return
that skipped both the versionless entry and its entire transitive
dependency subtree. This affected file: deps, workspace packages, and
linked packages. Now null version produces a versionless PURL and always
recurses into children, matching the JS client behavior.

Implements TC-4128

Assisted-by: Claude Code
@ruromero
fix: add missing test fixture for versionless root dep test
b9ff6e2 
The deps_with_no_version_root_dep fixture directory was not staged in the
previous commit, causing a NullPointerException in CI when the test tried
to load the resource files.

Implements TC-4128

Assisted-by: Claude Code
@ruromero
fix: align addRootDependenciesFromKey functionality
03c7132 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
ruromero merged commit cddf65b into guacsec:main Apr 20, 2026
37 of 44 checks passed
@ruromero
ruromero deleted the TC-4128 branch April 20, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@a-oren a-oren a-oren approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ruromero @a-oren